Skip to content

Template best practices#123

Merged
openshift-merge-bot[bot] merged 4 commits into
openshift-assisted:masterfrom
zszabo-rh:template_best_practices
Oct 17, 2025
Merged

Template best practices#123
openshift-merge-bot[bot] merged 4 commits into
openshift-assisted:masterfrom
zszabo-rh:template_best_practices

Conversation

@zszabo-rh
Copy link
Copy Markdown
Contributor

@zszabo-rh zszabo-rh commented Oct 13, 2025

Summary

By applying some best practices from the template MCP server implementation, this PR aims to modernize assisted-service-mcp to have modular and maintainable codebase with centralized configuration and clean, human-readable tool documentation. It preserves full backward compatibility while hopefully improving developer productivity and reliability.

  • Benefits:
    • Clear modular architecture (easier to extend and test)
    • Centralized, type-safe configuration with .env support
    • Consistent, human-readable tool docs with precise Annotated argument metadata

Commit 1 — Source organization and modular architecture

  • Refactored the monolithic server.py into a modular structure (server.py gets completely retired only in commit 3):
    • assisted_service_mcp/src/main.py: entrypoint; metrics setup; server start
    • assisted_service_mcp/src/api.py: FastAPI transport selection (SSE/streamable-http)
    • assisted_service_mcp/src/mcp.py: MCP server initialization; tool registration; wrapper injection
    • assisted_service_mcp/src/tools/: domain-oriented tool modules
    • assisted_service_mcp/utils/: utilities (auth.py, helpers.py, client_factory.py)
  • Kept top-level server.py as a thin wrapper for backward test compatibility.

Commit 2 — Centralized configuration (pydantic-settings) and tool description cleanup

  • Centralized configuration: added assisted_service_mcp/src/settings.py using pydantic-settings + python-dotenv
  • Tool descriptions refactor:
    • Consolidated docstrings into human-readable narrative with:
      • Description, Examples, Prerequisites, Related tools, Returns
    • Moved all argument details to Annotated Field(description=...) metadata; eliminated redundant Args sections

Commit 3 — Testing, coverage, and package relocation, and everything else

  • Further changes in package layout and imports: relocated metrics, static_net, and service_client under assisted_service_mcp/src/
  • Cleaned up modules not needed anymore (e.g. client_factory.py)
  • Fixes due to linter errors and review findings

All tests are green, server boots cleanly, tools are registered, and client-facing behavior remains unchanged.

Summary by CodeRabbit

  • New Features

    • New MCP package with selectable transport (SSE or streamable‑HTTP), many cluster/network/host/operator tools, authentication utilities, Prometheus metrics, and a module entrypoint for running the service.
  • Documentation

    • Added .env template and updated README/startup instructions to use the module entrypoint.
  • Refactor

    • Centralized runtime settings with validation and improved logging (sensitive-data redaction); updated local/container run targets and test coverage configuration.
  • Tests

    • Expanded test coverage across settings, auth, metrics, tools, transports, and log analysis.
  • Chores

    • Removed legacy server script and consolidated startup/config paths.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants